Skip to content

Conversation

@sridharavinash
Copy link
Contributor

@sridharavinash sridharavinash commented May 29, 2025

This pull request introduces significant updates to the authentication system for the MCP Registry Publisher Tool. The changes focus on transitioning to an interface-based authentication system, implementing GitHub OAuth device flow as the default authentication method, and restructuring the codebase for better modularity and extensibility.

Adding an interface to the publishing CLI, so that it'll be easier to add more auth methods as they become available.

@sridharavinash sridharavinash marked this pull request as ready for review May 29, 2025 21:29
@tadasant tadasant requested a review from Copilot May 30, 2025 14:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the authentication system for the MCP Registry Publisher Tool by introducing an interface‐based design and relying solely on GitHub OAuth device flow for authentication.

  • Introduce the auth.Provider interface and a corresponding GitHub OAuth implementation.
  • Remove legacy device flow login functions from main.go and add an "auth-method" flag for future extensibility.
  • Update documentation in both auth/README.md and publisher/README.md to reflect the new authentication approach.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tools/publisher/main.go Migrated authentication logic to use the new Provider interface
tools/publisher/auth/interface.go Introduced the Provider interface for authentication mechanisms
tools/publisher/auth/github/oauth.go Implemented GitHub OAuth using the device flow and token management
tools/publisher/auth/README.md Updated documentation to describe the interface-based auth system
tools/publisher/README.md Revised command-line arguments and important notes to align with changes
Comments suppressed due to low confidence (1)

tools/publisher/auth/README.md:21

  • The example usage in the README does not match the current constructor signature in oauth.go, which now expects (forceLogin, registryURL); please update the example accordingly.
- **Example**: github.NewOAuthProvider(clientID, forceLogin)

tadasant
tadasant previously approved these changes May 30, 2025
Copy link
Member

@tadasant tadasant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! Thank you for doing this!


The main application automatically selects the appropriate authentication provider:

1. If `--token` flag is provided, uses `TokenProvider`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel like Copilot should have commented here 😅

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the publisher tool to use an interface-based authentication system and implements GitHub OAuth device flow as the default auth method, improving modularity and extensibility.

  • Introduces an auth.Provider interface for pluggable authentication methods
  • Implements GitHub OAuth device flow in auth/github/oauth.go
  • Updates main.go and documentation to drive auth via the new interface

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tools/publisher/main.go Replaced inline device-flow logic with interface-driven auth
tools/publisher/auth/interface.go Added Provider interface for authentication mechanisms
tools/publisher/auth/github/oauth.go Created OAuthProvider implementing the interface
tools/publisher/auth/README.md Added docs for the new auth system (contains outdated examples)
tools/publisher/README.md Updated CLI flags and authentication overview

tadasant
tadasant previously approved these changes May 30, 2025
Copy link
Member

@tadasant tadasant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just Copilot docs nits

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the publisher CLI to use a pluggable authentication interface, implements GitHub OAuth device-flow as the default provider, and updates documentation accordingly.

  • Introduced auth.Provider interface and moved GitHub device-flow logic into github.OAuthProvider
  • Updated main.go to select and invoke the chosen auth provider instead of inline logic
  • Revised README files to describe the new auth architecture and CLI flag changes

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tools/publisher/main.go Refactor CLI to use auth.Provider, add --auth-method flag
tools/publisher/auth/interface.go Added Provider interface definition
tools/publisher/auth/github/oauth.go Implemented GitHub OAuth device-flow provider
tools/publisher/auth/README.md Documented the interface and provider setup
tools/publisher/README.md Updated command-line flag docs and simplified auth instructions
Comments suppressed due to low confidence (1)

tools/publisher/auth/interface.go:1

  • The new Provider interface and its implementations currently lack unit tests. Adding tests for both the interface contract and the GitHub OAuth provider will help ensure correct behavior and catch regressions.
package auth

@sridharavinash sridharavinash requested a review from Copilot May 30, 2025 17:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the publisher CLI to use an interface-based authentication system, extracts GitHub device-flow logic into a dedicated provider, and updates documentation and flags to support multiple auth methods.

  • Introduced auth.Provider interface and moved GitHub OAuth flow into auth/github/oauth.go
  • Refactored main.go to select and use an auth.Provider based on a new -auth-method flag
  • Updated README files to reflect new flag names, usage examples, and extensibility documentation

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tools/publisher/main.go Replaced inline device-flow code with auth.Provider usage
tools/publisher/auth/interface.go Added Provider interface definition
tools/publisher/auth/github/oauth.go Implemented OAuthProvider for GitHub device-flow OAuth
tools/publisher/auth/README.md Documented authentication architecture and extension steps
tools/publisher/README.md Updated CLI usage, flags, and authentication overview
Comments suppressed due to low confidence (2)

tools/publisher/main.go:11

  • The import block is missing required imports for flag, context, and log, and includes an unused import strings. Please add imports for "flag", "context", and "log", and remove "strings".
"strings"

tools/publisher/auth/github/oauth.go:40

  • No unit tests were added for OAuthProvider methods (NeedsLogin, Login, GetToken) or for getClientID. Adding tests will help ensure expected behavior and prevent regressions.
type OAuthProvider struct {

@sridharavinash sridharavinash merged commit a4cefcf into main May 30, 2025
10 checks passed
@sridharavinash sridharavinash deleted the publish-auth-interface branch May 30, 2025 19:05
domdomegg pushed a commit that referenced this pull request Aug 7, 2025
This pull request introduces significant updates to the authentication
system for the MCP Registry Publisher Tool. The changes focus on
transitioning to an interface-based authentication system, implementing
GitHub OAuth device flow as the default authentication method, and
restructuring the codebase for better modularity and extensibility.

Adding an interface to the publishing CLI, so that it'll be easier to
add more auth methods as they become available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants